Skip to content

Expand Glyphs multiple languages syntax when building UFOs - #1162

Open
tomekthewo wants to merge 8 commits into
googlefonts:mainfrom
tomekthewo:expand-multi-language-syntax
Open

Expand Glyphs multiple languages syntax when building UFOs#1162
tomekthewo wants to merge 8 commits into
googlefonts:mainfrom
tomekthewo:expand-multi-language-syntax

Conversation

@tomekthewo

@tomekthewo tomekthewo commented Sep 2, 2026

Copy link
Copy Markdown

Fixes #1109.

Glyphs lets a single language statement carry several tags and applies everything that follows to each of them:

language AZE CRT KAZ TAT TRK;
lookup idotaccent {
    sub i by idotaccent;
} idotaccent;

The FEA spec allows exactly one tag, so feaLib stops at the second one and the UFOs glyphsLib writes cannot be compiled at all — <features>:N:14: Expected ';', column 14 being where CRT starts. The syntax is documented in the Glyphs Handbook and written up as a spec proposal in adobe-type-tools/feature_file_workshops#8. Glyphs 4 emits it from automatic feature generation, so a source that built yesterday stops building after a save.

This expands the shorthand into one statement per tag in _to_ufo_features. What reaches the UFO is then plain spec-compliant FEA that any compiler reads — feaLib, fea-rs and makeotf alike; expanding downstream would leave the UFO itself non-portable.

The scope is emitted once for the first tag and replayed for every remaining one. Named lookups cannot simply be duplicated — that would redefine them — so the replays reference the lookup defined under the first tag:

language AZE;
lookup idotaccent {
    sub i by idotaccent;
} idotaccent;
language CRT;
lookup idotaccent;

Glyph class and markClass definitions are likewise emitted only once; bare rules and lookupflags are repeated verbatim, which is what the shorthand means. Trailing keywords (exclude_dflt and friends) are kept on every tag. A statement that is already spec-compliant, or that cannot be classified with confidence, is left untouched for feaLib to report rather than guessed at. So is a scope containing an include(), whose contents are not visible and therefore cannot be replayed; a statement listing dflt beside other tags, which has to be specified alone and which Glyphs rejects as well; one sitting outside any block, where FEA does not allow language at all; and one whose scope holds a statement with no closing semicolon.

Built on feaLib's Lexer. The scan walks the token stream, so a statement ends at the first ; at brace depth zero wherever it sits on the page, and a brace or semicolon inside a comment or a string is never taken for code. What is left is classifying five statement heads (lookup NAME {, @Class =, markClass, include(, everything else): no glyph classes to resolve, no lookup bodies to understand, no name resolution. What is emitted for the first tag is sliced out of the original by offset, so comments, indentation and blank lines survive untouched; the replays are statements only, so a comment inside the scope is not duplicated. A \blanguage\b guard keeps the lexer off feature code that cannot contain the shorthand.

It reproduces what Glyphs 3 wrote. Taking a locl feature from a production source in both its Glyphs 3 and its Glyphs 4 form — the automatic code nobody edited, before and after a save in Glyphs 4 — expanding the shorthand gives back the Glyphs 3 text byte for byte, tabs and blank lines included. Compiled with feaLib, the two produce an identical GSUB table. A second test does the same for the shape where the language statements sit inside the lookup block; there the single-tag form is what Glyphs 3 wrote for another production source and the shorthand is that form collapsed by hand, since Glyphs 4 has not been seen to emit it — it pins down that the scope ends at the block's own brace, with the rules repeated rather than referenced.

Ordering. The expansion runs after VariableFeatureConverter, so #ifndef VARIABLE blocks are already resolved. The markers are comments and therefore not statements: expanding first would place the repeats inside the block, and the strip that follows would then take the second language statement with it. test_conditional_block_is_resolved_before_the_expansion pins this down.

One-way. The expansion is not reversed on the way back, so .glyphs → UFO → .glyphs yields the expanded form rather than the original statement. test_expansion_does_not_round_trip pins this down. UFO → .glyphs → UFO is unaffected, since the original feature text is recovered from ORIGINAL_FEATURE_CODE_KEY. Happy to restore the shorthand in to_glyphs_features instead if you would rather keep it lossless.

39 tests in tests/builder/multi_language_test.py, including the four counterexamples from the review, the two real-source shapes above, every line ending feaLib recognises, each shape left untouched, idempotence, two through to_ufos and one through the round trip. Two are invariants over a corpus of awkward sources rather than one example each: the output is either identical to the input or it compiles, and line endings do not change which rules end up under which tag. Full suite: 1376 passed, 64 skipped, 4 xfailed. black --check and flake8 over Lib tests are clean.

Glyphs lets a single `language` statement carry several tags and applies
everything that follows to each of them, which the FEA spec does not allow, so
feaLib rejects the generated features with `Expected ';'` at the second tag and
the source cannot be compiled with fontmake at all. Expand the shorthand into
one statement per tag on the way to the UFO, next to the variable feature
conversion.

The block is emitted once for the first tag and repeated for every remaining
one. Named lookups cannot simply be duplicated - that would redefine them - so
the repeats reference the lookup defined under the first tag, and glyph class
definitions are likewise emitted only once; bare rules and lookupflags are
repeated verbatim. Trailing keywords are kept on every tag. A statement that is
already spec-compliant, or that cannot be parsed with confidence, is left
untouched for feaLib to report.

Fixes googlefonts#1109
The line scanner counted braces and matched statements on the raw text, so
anything inside a comment was taken for code. A `}` in a comment ended the
block early and bound the following rules to the last tag only -- output that
parses cleanly and ships the wrong `locl` mapping -- while a `{` swallowed the
feature's own closing brace. Statements with a trailing comment matched neither
the delimiter nor the language pattern, so they were absorbed into the previous
body and replayed, and a multi-tag statement with a trailing comment was not
expanded at all.

Blank out comments and string literals before matching, keeping column numbers
so the original lines are still what gets emitted, and carry a trailing comment
over to the first expanded statement.

Also handle three shapes that were treated as ordinary lines and so duplicated
rather than referenced or dropped: `lookup NAME useExtension {`, a lookup whose
brace is on the next line, and `markClass` definitions. Glyph class definitions
now run to their terminating semicolon instead of one line, so a multi-line
class no longer leaves orphan tokens behind.
variable_features.py already blanks comments and string literals before
scanning feature code; use it instead of a second copy. Also compile the
language tag pattern at module level like the others, and name the brace depth
calculation the two scanners share.
Comment thread tests/builder/multi_language_test.py Outdated
lines = stripped(fea)
assert lines.count("sub Scedilla by Scommaaccent;") == 2
assert lines.count("language ROM;") == 1
assert lines.count("language MOL;") == 1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test the whole output against an expectation. Counting matching lines does not guarantee the output is as expected.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, done in b354bdb — every case now compares the full expanded text, which doubles as documentation of what the expansion produces. It caught nothing new, but it is a much better regression net: the counting assertions would not have noticed the order of the emitted statements. The two round-trip tests assert the whole feature text as well.

Counting matching lines does not pin down the structure or the order of the
result. Compare the full text instead, which also documents what the expansion
produces.
@anthrotype

Copy link
Copy Markdown
Member

So.. I’m not convinced a physical line is the right unit for this transformation. FEA defines line endings as ordinary whitespace therefore valid statements may span lines or share one.

Unlike with the existing variable_features.py, expanding multiple languages cannot operate on locally delimited syntax because it needs to understand the type and scope of the following FEA statements, which means reconstructing statement boundaries, nesting, definitions and lookup references, all through a bunch of fragile regexes...

E.g. this would be valid Glyphs syntax that the current code leaves unexpanded because another statement follows on the same line:

language AZE CRT; sub i by idotaccent;

Or take this one:

language AZE CRT;
lookup idot { sub i by idotaccent; } idot;
sub Scedilla by Scommaaccent;

here the scanner can't detect a lookup's opening and closing braces that occur on the same line (brace depth is 0) and treats the lookup as continuing to the end of the language section, so CRT only receives lookup idot; while the inline sub rule gets silently ignored.

There are more of these all stemming from conflating line with statements. E.g. @Ced = [Scedilla]; sub @Ced by Scommaaccent; would be classified as one definition and the sub not replayed. Or line break after lookup or markClass would prevent the definition from being recognized.

Fixing these (or even just rejecting them with a loud error to avoid silent wrong output) risks turning this into a partial second FEA parser inside glyphsLib, needing knowledge of lookup, class/markClass semantics. Besides, this operates on incomplete feature text before includes gets resolved, so there's that too.

So I’m not yet convinced glyphsLib can transform the extension safely without duplicating parts of feaLib. And maybe feaLib would not necessarily be the wrong layer, despite not being official FEA... I honestly don't know, sorry for being inconclusive.

@anthrotype

Copy link
Copy Markdown
Member

(and then there is the question of also needing to implement this in fontc/fea-rs which just hit 1.0 this week.. at some point I would rather stop introducing new features in both compilers)

@tomekthewo

Copy link
Copy Markdown
Author

You're right, and the line-based scanning isn't salvageable. Thanks a lot for writing the counterexamples out — all four reproduced exactly as you described, including the two that fail silently.

I've rewritten the expansion on feaLib's own Lexer instead of on the text:

input before now
language AZE CRT; sub i by idotaccent; not expanded expanded
lookup idot { … } idot; closed on one line CRT lost the rule that followed both statements replayed
@Ced = [Scedilla]; sub @Ced by Scommaaccent; CRT got nothing rule replayed, class defined once
line break after lookup block duplicated → redefinition replayed as lookup idot;

The part I'd gently push back on is the "partial second FEA parser" worry — tokenizing is precisely what removes the need for one. Lexer already gives me statement boundaries (a ; at brace depth zero, wherever it sits on the page), and it already tells a brace inside a comment or a string from a real one. What's left is classifying the head of each statement, and that grammar is five cases:

  • lookup NAME [useExtension] { → definition: emitted once, replayed as lookup NAME;
  • @Class = … → definition: emitted once
  • markClass … → definition: emitted once
  • include(…) → bail out (below)
  • anything else, lookup NAME; references included → repeated verbatim

with the scope ending at the next language/script or an unmatched }. No glyph classes to resolve, no lookup bodies to understand, no name resolution. The emitted text is sliced out of the original by offset, so comments, indentation and blank lines survive untouched.

TokenExpander in builder/tokens.py already runs inside _to_ufo_features and rewrites Glyphs-only $[…] / ${…} / $name syntax into plain FEA — with a hand-rolled predicate subparser and an eval() for the arithmetic. In my opinion, it is more Glyphs-specific machinery than the five heads above. I know those tokens are locally delimited and the shorthand isn't — that's a fair distinction. My point is only that lexing moves that line: on a token stream a statement boundary is as local as the } that closes a ${.

On includes: agreed, they can't be replayed, so a multi-tag scope containing one is left alone entirely. The shorthand is invalid FEA, so feaLib then reports it — the failure stays loud instead of becoming a silently partial expansion.

What this does not address is your second comment about fontc parity, because it still fails unless the same expansion is implemented there too. The portability argument in my PR description ("the UFO gets plain FEA, so any compiler reads it") doesn't cover fontc, and I shouldn't have framed it that broadly. The parity cost you're describing is real and this patch doesn't remove it.

One thing that might change the calculus: Glyphs 4 emits it from automatic feature generation. I took two of our Glyphs 3 sources (appVersion 3517 and 3526), opened and saved each in Glyphs 4 (appVersion 4004), and in both the automatic locl came back rewritten the same way:

# before, appVersion 3517
language AZE;
lookup locl_latn_0 {
    sub i by idotaccent;
} locl_latn_0;
language CRT;
lookup locl_latn_0;
language KAZ;
lookup locl_latn_0;
...

# after, appVersion 4004
language AZE CRT KAZ TAT TRK;
lookup idotaccent {
    sub i by idotaccent;
} idotaccent;

No edits by the designer, the feature still flagged automatic before and after, and both fonts went from ten single-tag statements to two single plus the same two shorthand ones (AZE CRT KAZ TAT TRK and ROM MOL). So this isn't an opt-in extension that some sources happen to use — it's ordinary Glyphs 4 output, and a source that built yesterday stops building after a save, in feaLib and fea-rs alike. Two fonts against one Glyphs 4 build is still not a survey, but it reproduces rather than looking like a fluke.

As I read it, there are three ways this could go:

  1. Token-based expansion as described — I can push it to this branch.
  2. Detect and reject, no expansion. A language statement with more than one tag becomes a clear error instead of Expected ';' pointing at column 14. Needs no statement semantics at all, needs no fontc counterpart, and is strictly better than today.
  3. Drop it — it belongs in feaLib/fea-rs, or nowhere.

I can only guess which one you'd prefer. But if Glyphs emits the shorthand on save, everything that reads .glyphs sources ends up having to solve it, and for the UFO path this is the one place to do it once — fontc would still need its own, which is the cost you flagged, though I don't think that argues for nobody doing it.

Happy to push (1) here, or cut it back to (2) — whichever you'd merge. For what it's worth on the approach itself: I ran the tokenizer over ~1100 feature code blocks from 35 production .glyphs files and all of them lex cleanly on the raw pre-include text, which is what convinced me it holds up outside the test suite.

@anthrotype

Copy link
Copy Markdown
Member

Happy to push (1) here

thanks, since you have done it already and sounds like it's better than the line-base scanner, you may as well push it

Glyphs 4 emits it from automatic feature generation

oh, great. That means we can't dismiss this and will have to support it in fontc anyway (googlefonts/fontc#2126). And maybe in feaLib as well, which would make the current PR redundant.

@schriftgestalt

Copy link
Copy Markdown
Collaborator

We documented our fea additions. There is a branch for each topic: https://github.com/schriftgestalt/feature_file_workshops (This is not meant as a request to add it to the spec as is, just to have written it down properly.)

@anthrotype

Copy link
Copy Markdown
Member

Thanks Georg, I briefly checked Adobe's upstream repo the other day but couldn't find your multi-lang extension proposal. I found it now here: adobe-type-tools/feature_file_workshops#8

@tomekthewo

Copy link
Copy Markdown
Author

Pushed. The line scanner is gone — multi_language.py now walks feaLib's own Lexer, so a statement ends at the first ; at brace depth zero wherever it sits on the page, and a brace inside a comment or a string is never code. Your four counterexamples are in the tests under their own names:

  • test_statement_sharing_the_line_with_the_shorthand_is_expanded
  • test_lookup_closed_on_one_line_does_not_swallow_what_follows
  • test_definition_sharing_a_line_with_a_rule_replays_the_rule
  • test_line_break_after_lookup_keyword_is_still_a_definition

Georg — thanks, adobe-type-tools/feature_file_workshops#8 is exactly the grammar this implements (language <tag>+ [exclude_dflt|include_dflt] [required];, following rules registered under every listed tag for the current script), so nothing to change here. And if the conclusion is that this belongs in feaLib rather than at the UFO boundary, that makes sense to me — expanding here is only worth it for as long as the compilers don't.

@anthrotype

Copy link
Copy Markdown
Member

Thanks.

adobe-type-tools/feature_file_workshops#8 is exactly the grammar this implements

What does Glyphs 4 do with dflt inside a multi-tag language statement? The proposal says it must be specified alone (https://github.com/schriftgestalt/feature_file_workshops/blob/2d1d7a058e2642a9bf6ef0f6dd8594ea7d7d40e8/OpenTypeFeatureFileSpecification.md?plain=1#L1219), and Glyphs 3.4.1 refuses to export it.

E.g. with the following feature prefix:

languagesystem DFLT dflt;
languagesystem latn dflt;
languagesystem latn AZE;

and this locl feature:

script latn;
language dflt AZE;
sub i by idotaccent;

Glyphs 3.4.1 says Error: Regular: Expected ";" after language statement, found in feature "locl", line 2.

Your PR expands it anyway, to language dflt; + the original rules and then language AZE; + the replayed rules.
But since language AZE; implies include_dflt, AZE gets the substitution twice, once inherited from dflt and once replayed, as two distinct lookups with identical content both registered under AZE. This is harmless for shaping but superfluous.

Could you check what Glyphs 4 does with it? If it rejects it too, I'd leave such a statement untouched and let feaLib report it, like the other shapes you don't classify. If it accepts it, knowing what lands in the GSUB would settle what the expansion should emit.

Scanning physical lines conflated lines with statements: a rule sharing
the line with the shorthand was not expanded, a lookup opened and closed
on one line swallowed what followed it, a definition sharing a line with
a rule dropped the rule from the repeats, and a line break after the
lookup keyword hid the definition. Walk feaLib's token stream instead,
where a statement ends at the first semicolon at brace depth zero and a
brace inside a comment or a string is not code.
The `#ifndef VARIABLE` markers are comments, so they are not statements
and are not replayed. Expanding first therefore placed the repeats
inside the block, and the strip that follows took the second `language`
statement with it, leaving the rules bound to the first tag alone.
Four shapes the scan handled badly, all now left for feaLib to report:

- `dflt` beside other tags. It has to be specified alone, and Glyphs
  rejects it: 4.0.1 (4004) and 3.4.1 both say `Expected ";" after
  language statement`. Expanding it compiled, but `language AZE;`
  implies `include_dflt`, so AZE carried the rules twice.
- A lone `\r`. feaLib ends a line at `\r`, at `\n` and at the pair;
  `_line_starts` split on `\n` only, so one carriage return shifted
  every later slice. The replay lost its rules while the output stayed
  valid FEA -- the one silently wrong case.
- A missing semicolon. `_statement_end` ran to the end of the token
  stream, or out through the brace closing the block, and the replay
  then repeated the following feature wholesale.
- A shorthand outside any block. FEA does not allow `language` there
  and its scope had no enclosing brace to end at, so it swallowed the
  block that followed.

`except IndexError` in `_tokenize` only masked the `\r` bug and is gone.

Two invariants over a corpus of awkward sources replace the
example-by-example coverage: the output is either identical to the
input or it compiles, and line endings do not change which rules end up
under which tag. Both fail on the previous commit.

Three existing tests now wrap their snippet in a `feature` block. The
entry point only ever sees the full assembled text, where a top-level
`language` is invalid, so those bare snippets pinned down a shape that
cannot occur.
@tomekthewo
tomekthewo force-pushed the expand-multi-language-syntax branch from 5507056 to 751b977 Compare September 7, 2026 13:49
@tomekthewo

Copy link
Copy Markdown
Author

Glyphs 4.0.1 (4004) rejects it too, with the same error as 3.4.1 — this is the compiler in the Features tab:

script latn;
language dflt AZE;
sub i by idotaccent;

Expected ";" after language statement on line 2, locl marked as failing.

So a multi-tag statement listing dflt is now left untouched, with a warning saying why, and feaLib reports it as before. Only dflt is guarded, not DFLT — expanding a DFLT statement still fails loudly in feaLib, while expanding dflt compiled and produced the redundant duplicate you described.

Looking for the same class of problem elsewhere turned up three more — a lone \r (feaLib ends a line there too, so the offsets went out of step and the replay silently lost its rules), a missing semicolon, and a shorthand outside any block — all now left alone, with two invariants over a corpus of awkward sources in place of one example each. Three existing tests wrap their snippet in a feature block now, since the entry point only ever sees the full assembled text, where a top-level language is invalid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent language tags aren't correctly recognised

4 participants